-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor classes to use __init__ #139
base: main
Are you sure you want to change the base?
Conversation
I believe we previously used __new__ to support Pylance. Now Pylance doesn't support __new__ so we need to use __init__. Mypy works either way, but this allows us to get Pylance working properly with auto complete.
oh pyright is angry in CI, maybe it needs an update? |
Using __init__ with array types causes a bunch of unknowns with Pylance.
def __new__( # type: ignore [misc] | ||
cls, | ||
def __init__( | ||
self: JSONField[_A], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont believe an annotation is needed for self
. At least I never see it in typeshed repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the annotation is required here to handle the Optional case. You can find examples of this in the typeshed repository too:
any eta? |
@bschnurr We can make this change, but it breaks foreign keys for Pylance. Mypy still works though: #136 (comment) |
@chdsbd found an alternative fix.. just remove
then the previous
pyright does have a check for this. |
this is a pylance bug. working on a fix |
I believe we previously used new to support Pylance. Now Pylance doesn't support new so we need to use init.
Mypy works either way, but this allows us to get Pylance working properly with auto complete.
related: #136